Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added caching to LayerManager #1734

Merged
merged 2 commits into from
Oct 15, 2021

Conversation

jonjondev
Copy link
Contributor

This PR adds caching to the LayerManager node to avoid some very expensive Maya queries being run when editing begins.

@kxl-adsk
Copy link

kxl-adsk commented Oct 1, 2021

I saw you have a number of fixes around LayerManager. Some of them were reasons for us to create a new copy of LayerManager in the core library (e.g. we needed all anonymous layers to be serialized to Maya file and not only session layer). Do you think with the upcoming changes you could actually adopt core LayerManager? The big benefit of going this path would help with migration from AL to Adsk plugin, fixes will benefit both plugins...and ultimately remove code duplication.

@jonjondev
Copy link
Contributor Author

Hi @kxl-adsk!

Again, this PR is part of AL's alignment process, and will likely have further changes applied on top soon. I think that adopting the shared LayerManager is moving the right direction, and I'll bring it up with the team. Similarly to #1731, let me know if there's anything concerning otherwise, but this purely reflects the effort to catch up with the latest internal changes in a somewhat linear manner.

@kxl-adsk
Copy link

I understand that this change only impacts the AL plugin. It also shouldn't make the transition to a common plugin any harder, so no concerns here. What makes me hesitate is the work you will have to put in adopting the shared LayerManager and I wouldn't want this to force us to keep both in the source tree. One way to make this transition easier would be to validate changes against the shared LayerManager and apply them to two places if possible/necessary.

Let's take this PR as an example - the same change can be made in https://github.com/Autodesk/maya-usd/blob/dev/lib/mayaUsd/nodes/layerManager.cpp#L137.

The benefit of paying this extra cost now and applying fixes to both LayerManagers is an easier transition to a shared LayerManager in (hopefully) the near future.

@jonjondev
Copy link
Contributor Author

Can do @kxl-adsk. Will make the appropriate changes to the shared layerManager and add it to the PR.

@kxl-adsk kxl-adsk merged commit 6b716c4 into Autodesk:dev Oct 15, 2021
@jonjondev jonjondev deleted the J-Mo63/cache-layer-manager branch October 17, 2021 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants